Restructure TVF#864
Merged
svchb merged 29 commits intotrixi-framework:mainfrom Aug 25, 2025
Merged
Conversation
efaulhaber
commented
Jul 21, 2025
Merged
LasNikas
requested changes
Aug 1, 2025
svchb
requested changes
Aug 1, 2025
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #864 +/- ##
==========================================
+ Coverage 71.00% 71.05% +0.04%
==========================================
Files 107 107
Lines 7043 7016 -27
==========================================
- Hits 5001 4985 -16
+ Misses 2042 2031 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
svchb
requested changes
Aug 2, 2025
svchb
requested changes
Aug 2, 2025
Co-authored-by: Sven Berger <berger.sven@gmail.com>
Collaborator
|
Can you add the continuity equation based on the advection velocity? At least in my testing this was much better! |
Member
Author
|
This is step 2 that I described above. It will be the next PR. This PR doesn't change features, it only refactors the code and fixes a bug. |
Member
Author
|
/run-gpu-tests |
svchb
previously approved these changes
Aug 13, 2025
Collaborator
|
/run-gpu-tests |
This was referenced Aug 16, 2025
Member
Author
|
/run-gpu-tests |
Collaborator
|
/run-gpu-tests |
svchb
approved these changes
Aug 18, 2025
LasNikas
approved these changes
Aug 25, 2025
Collaborator
|
/run-gpu-tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


This PR consists of several changes that I did in steps.
Step 1
The current approach is integrating the transport velocity through stages. So, between time steps,

\tilde{v}is set tov, and then it is integrated for one time step withd\tilde{v}/dtset to the term inside the parentheses here:Note that this approach is currently flawed, as the
dv/dtterm in the RHS is missing, so only the physical velocity from the last time step is used at every stage.In the paper, with the different time integration, this transport velocity is only computed once per step:

While we can interpret this as "transport velocity is integrated within the step starting from v", which leads to the approach explained above, we could also interpret this as "
dr/dt = v + δv, whereδvisdt * v_candv_cis the second term inside the parentheses above".This interpretation leads to a
δv = dt * v_cthat is either constant throughout the time step and computed between steps, orv_cis recomputed at every stage. The latter is the one I implemented for the following comparisons, but I don't think the difference between computing this only between steps is significant, so we might be able to save some significant computation here.Results
Step 2
The original 2013 paper was using kernel summation to compute the density. When we use this implementation with the continuity equation, we get the same inconsistency as the inconsistent PST, as particles are now moving with the transport velocity, but the physical velocity is used in the continuity equation.
This can lead to an uncontrolled increase of density in a closed simulation.
In "A generalized transport-velocity formulation for smoothed particle hydrodynamics" (Zhang et al. 2017), the density is integrated with the continuity equation and the transport velocity is used here:
